[libc] Simplify fma handling for riscv #149673
Merged
+5
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR simplifies how we enable the different fma configs for riscv: 1. Removes __LIBC_RISCV_USE_FMA define 2. Checks if __riscv_flen is defined to set LIBC_TARGET_CPU_HAS_FMA As a bonus we enable *FMA_OPT tests for rv32, so any rv32 hardware that doesn't implement the f/d extensions are also tested.
|
@llvm/pr-subscribers-libc Author: Mikhail R. Gadelha (mikhailramalho) ChangesThis PR simplifies how we enable the different fma configs for riscv:
As a bonus, we enable *FMA_OPT tests for rv32, so any rv32 hardware that doesn't implement the f/d extensions is also covered by the tests. Full diff: https://github.com/llvm/llvm-project/pull/149673.diff 4 Files Affected:
diff --git a/libc/cmake/modules/LLVMLibCArchitectures.cmake b/libc/cmake/modules/LLVMLibCArchitectures.cmake
index c94a407d974df..69487cbca225d 100644
--- a/libc/cmake/modules/LLVMLibCArchitectures.cmake
+++ b/libc/cmake/modules/LLVMLibCArchitectures.cmake
@@ -153,9 +153,11 @@ elseif(LIBC_TARGET_ARCHITECTURE STREQUAL "x86_64")
elseif(LIBC_TARGET_ARCHITECTURE STREQUAL "i386")
set(LIBC_TARGET_ARCHITECTURE_IS_X86 TRUE)
elseif(LIBC_TARGET_ARCHITECTURE STREQUAL "riscv64")
+set(LIBC_TARGET_ARCHITECTURE_IS_ANY_RISCV TRUE)
set(LIBC_TARGET_ARCHITECTURE_IS_RISCV64 TRUE)
set(LIBC_TARGET_ARCHITECTURE "riscv")
elseif(LIBC_TARGET_ARCHITECTURE STREQUAL "riscv32")
+set(LIBC_TARGET_ARCHITECTURE_IS_ANY_RISCV TRUE)
set(LIBC_TARGET_ARCHITECTURE_IS_RISCV32 TRUE)
set(LIBC_TARGET_ARCHITECTURE "riscv")
elseif(LIBC_TARGET_ARCHITECTURE STREQUAL "amdgpu")
diff --git a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
index 82d06e2b9eb55..2478fde64d430 100644
--- a/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
+++ b/libc/cmake/modules/LLVMLibCCompileOptionRules.cmake
@@ -13,7 +13,7 @@ endif()
function(_get_compile_options_from_flags output_var)
set(compile_options "")
- if(LIBC_TARGET_ARCHITECTURE_IS_RISCV64 OR(LIBC_CPU_FEATURES MATCHES "FMA"))
+ if(LIBC_CPU_FEATURES MATCHES "FMA")
check_flag(ADD_FMA_FLAG ${FMA_OPT_FLAG} ${ARGN})
endif()
check_flag(ADD_ROUND_OPT_FLAG ${ROUND_OPT_FLAG} ${ARGN})
@@ -25,8 +25,6 @@ function(_get_compile_options_from_flags output_var)
if(LIBC_TARGET_ARCHITECTURE_IS_X86_64)
list(APPEND compile_options "-mavx2")
list(APPEND compile_options "-mfma")
- elseif(LIBC_TARGET_ARCHITECTURE_IS_RISCV64)
- list(APPEND compile_options "-D__LIBC_RISCV_USE_FMA")
endif()
# For clang, we will build the math functions with `-fno-math-errno` so that
# __builtin_fma* will generate the fused-mutliply-add instructions. We
diff --git a/libc/cmake/modules/LLVMLibCFlagRules.cmake b/libc/cmake/modules/LLVMLibCFlagRules.cmake
index 7d5e73c2f1214..4bbd21ab569dc 100644
--- a/libc/cmake/modules/LLVMLibCFlagRules.cmake
+++ b/libc/cmake/modules/LLVMLibCFlagRules.cmake
@@ -270,7 +270,7 @@ set(MISC_MATH_BASIC_OPS_OPT_FLAG "MISC_MATH_BASIC_OPS_OPT")
# Skip FMA_OPT flag for targets that don't support fma.
if(NOT DEFINED SKIP_FLAG_EXPANSION_FMA_OPT)
if(NOT((LIBC_TARGET_ARCHITECTURE_IS_X86_64 AND (LIBC_CPU_FEATURES MATCHES "FMA")) OR
- LIBC_TARGET_ARCHITECTURE_IS_RISCV64))
+ LIBC_TARGET_ARCHITECTURE_IS_ANY_RISCV))
set(SKIP_FLAG_EXPANSION_FMA_OPT TRUE)
endif()
endif()
diff --git a/libc/src/__support/macros/properties/cpu_features.h b/libc/src/__support/macros/properties/cpu_features.h
index cdb2df97b2b9a..fde30eadfd83b 100644
--- a/libc/src/__support/macros/properties/cpu_features.h
+++ b/libc/src/__support/macros/properties/cpu_features.h
@@ -81,7 +81,7 @@
#endif
#if defined(__ARM_FEATURE_FMA) || (defined(__AVX2__) && defined(__FMA__)) || \
- defined(__NVPTX__) || defined(__AMDGPU__) || defined(__LIBC_RISCV_USE_FMA)
+ defined(__NVPTX__) || defined(__AMDGPU__) || defined(__riscv_flen)
#define LIBC_TARGET_CPU_HAS_FMA
// Provide a more fine-grained control of FMA instruction for ARM targets.
#if defined(LIBC_TARGET_CPU_HAS_FPU_HALF)
|
lntue
reviewed
Jul 19, 2025
lntue
reviewed
Jul 19, 2025
lntue
approved these changes
Jul 19, 2025
Signed-off-by: Mikhail R. Gadelha <[email protected]>
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/38694 Here is the relevant piece of the build log for the reference |
This was referenced Jul 23, 2025
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Jul 28, 2025
This PR simplifies how we enable the different fma configs for riscv: 1. Removes __LIBC_RISCV_USE_FMA define 2. Checks if __riscv_flen is defined to set LIBC_TARGET_CPU_HAS_FMA As a bonus, we enable *FMA_OPT tests for rv32, so any rv32 hardware that doesn't implement the f/d extensions is also covered by the tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR simplifies how we enable the different fma configs for riscv:
As a bonus, we enable *FMA_OPT tests for rv32, so any rv32 hardware that doesn't implement the f/d extensions is also covered by the tests.